Skip to content

[Experimental zone for RFC FS-1043: Extension members participate in SRTP constraint resolution]#19396

Draft
T-Gro wants to merge 163 commits intomainfrom
feature-operators-extensions
Draft

[Experimental zone for RFC FS-1043: Extension members participate in SRTP constraint resolution]#19396
T-Gro wants to merge 163 commits intomainfrom
feature-operators-extensions

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented Mar 6, 2026

Implements RFC FS-1043. Gated behind --langversion:preview.

See docs/RFC_Changes.md for implementation-specific amendments to the RFC (interop, diagnostics, compatibility, binary compat).

1. Extension members solve SRTP constraints

Extrinsic extension members (operators, named methods, properties) in scope at an inline function's definition site are now captured in the SRTP constraint and considered during overload resolution. Built-in primitive operator solutions retain priority when types match precisely.

type System.String with
    static member (*) (s, n: int) = String.replicate n s

let r = "ha" * 3  // "hahaha" — previously FS0001

2. Weak resolution deferred for inline code

Inline SRTP functions no longer eagerly collapse to a concrete type. let inline f1 (x: DateTime) y = x + y stays generic (DateTime -> ^a -> ^b when ...). Breaking: signature files and monomorphic bindings may need updating.

3. [<AllowOverloadOnReturnType>]

New FSharp.Core attribute enables return-type-based overload resolution for any method, extending behavior previously reserved for op_Explicit/op_Implicit.

T-Gro and others added 30 commits February 13, 2026 20:30
- Add LanguageFeature.ExtensionConstraintSolutions mapped to preview version
- Gate FS1215 (tcMemberOperatorDefinitionInExtrinsic) behind the feature flag
- With --langversion:preview, extension operator definitions no longer warn
- Add FSComp.txt entry and xlf translations for the feature description
…with traitCtxt field

- Add ITraitAccessorDomain marker interface to TypedTree
- Add ITraitContext interface with SelectExtensionMethods and AccessRights
- Extend TTrait with 8th field: traitCtxt: ITraitContext option
- Add TraitContext member and traitCtxtNone helper
- Make AccessorDomain implement ITraitAccessorDomain
- Update all TTrait pattern matches across the compiler (8 files)
- Pickling: traitCtxt is not serialized; deserialized as None
- No behavioral change: all trait contexts are None

Sprint 2 of RFC FS-1043 (extension method SRTP resolution)
…functions

- Add traitCtxt: ITraitContext option parameter to CopyTyparConstraints,
  FixupNewTypars, FreshenAndFixupTypars, FreshenTypeInst, FreshMethInst,
  FreshenMethInfo, FreshenTypars, CopyAndFixupTypars, FreshenPossibleForallTy,
  LightweightTcValForUsingInBuildMethodCall, FreshenTyconRef, FreshenTyconRef2,
  FreshenAbstractSlot, FreshenObjectArgType
- CopyTyparConstraints stamps traitCtxt onto MayResolveMember constraints
  when the original has None
- TcEnv implements ITraitContext with SelectExtensionMethods and AccessRights
- Add SelectExtensionMethInfosForTrait to NameResolution.fs
- Thread traitCtxtNone through all call sites (no behavioral change)
…l sites with TcEnv in scope

- TTrait construction sites in TcPseudoMemberSpec and TcImplicitOpItemThen
  now use env.TraitContext instead of None
- All 28 freshening call sites in CheckExpressions.fs use env.TraitContext
  (or envinner.TraitContext where the variable is named envinner)
- CheckDeclarations.fs call sites use env/envForTycon/tcEnv.TraitContext
- CheckExpressionsOps.fs CompilePatternForMatch uses env.TraitContext
- CopyTyparConstraints traitCtxt propagation is no longer dead code
- GetRelevantMethodsForTrait: collect extension methods from trait context
  when ExtensionConstraintSolutions feature is enabled
- SolveMemberConstraint: compute traitAD from trait context for accessibility
- Use traitAD in CalledMeth, ResolveOverloading, TryFindIntrinsicNamedItemOfType
- Use traitCtxt (not traitCtxtNone) in FreshenMethInfo calls
- Built-in rules use intrinsicMinfos in match pattern; when clauses use full
  minfos so extension methods properly defer built-in resolution
- traitsAEquiv already ignores traitCtxt field (verified)
… tests

- Change typecheck to compileAndRun in 'Extension operator on string resolves
  with langversion preview' test to validate runtime behavior
- Add test: FS1215 warning fires for extension operator without langversion preview
- Add test: FS1215 warning does not fire with langversion preview

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ring optimization

When FSharp.Core inline operators are expanded, the resulting trait calls have
no ITraitContext (it's not serialized in metadata). This prevented extension
methods defined in the current compilation unit from being found during
optimization/codegen, causing Error 193 type mismatches.

Changes:
- ConstraintSolver: Add guard preventing built-in mul/div rule from firing
  for concrete non-numeric types when ExtensionConstraintSolutions is enabled
- ConstraintSolver: Add CreateImplFileTraitContext that walks the expression
  tree bindings (not CcuThunk module type) to find extension member Val objects
  with correct stamps matching IlxGen's bound vals
- Optimizer: Add traitCtxt field to cenv, supply fallback trait context in
  OptimizeTraitCall when trait's original context is None

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test B5: Multiple extension operators with different signatures resolve correctly
- Test B6: Intrinsic operator takes priority over extension with same name and signature

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 5 new tests covering:
- Extension operator resolves via SRTP alongside IWSAM types (B7)
- Extension wins over IWSAM interface impl for same operator (B7)
- Extension operator not visible without opening defining module (B9)
- Inline SRTP resolves using consumer's scope for extensions (B9)
- Internal record field resolves via SRTP within same compilation unit (B10)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test 1: True optional extension on System.String (*) across assemblies.
  Documents that cross-assembly extension resolution fails (Error 193)
  because TTrait.traitCtxt deserializes as None from pickled metadata.
- Test 2: Intrinsic augmentation Widget (+) across assemblies.
  Confirms intrinsic operators work cross-assembly (for contrast).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove three tests that were near-identical duplicates exercising the same
'define record + extension (+) + inline add' pattern:
- 'Inline SRTP function uses extension method on custom type' (duplicate of 1910)
- 'Extension operator resolves via SRTP alongside IWSAM types' (duplicate, misleading name)
- 'Extension operator on custom type typechecks in separate module' (subset of Intrinsic method priority test)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add defensive type-test assertions replacing unsafe obj downcasts in
  CheckBasics.fs (InfoReader), ConstraintSolver.fs (AccessorDomain, MethInfo)
- Hoist ExtensionConstraintSolutions feature flag check to local binding
  in SolveMemberConstraint and GetRelevantMethodsForTrait
- Extract SelectExtMethInfosForType shared helper in NameResolution.fs
  and refactor SelectExtensionMethInfosForTrait to use it

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… resolution

Define AllowOverloadOnReturnTypeAttribute in FSharp.Core and wire it into
the compiler's overload resolution and uniqueness checking. When any
applicable overload carries this attribute, the return type is considered
during overload resolution, generalizing the existing op_Explicit/op_Implicit
mechanism to arbitrary methods.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eftover files

Update FSharp.Core surface area baseline to include the new
AllowOverloadOnReturnTypeAttribute type. Remove leftover code review
artifact files that were accidentally committed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…InfosForType

Extract SelectIndexedExtMethInfosForType helper for indexed-only extension
member lookup. Refactor ExtensionMethInfosOfTypeInScope to use
SelectExtMethInfosForType for the root type (both indexed and unindexed
members) and SelectIndexedExtMethInfosForType for base types in the
hierarchy (indexed only), removing the duplicated tryTcrefOfAppTy /
eIndexedExtensionMembers.Find / SelectMethInfosFromExtMembers pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sForType

Eliminate code duplication by having SelectExtMethInfosForType delegate its
indexed lookup to SelectIndexedExtMethInfosForType instead of duplicating
the tryTcrefOfAppTy/Find/SelectMethInfosFromExtMembers pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ions

Skip SRTP constraint canonicalization for inline bindings when the
ExtensionConstraintSolutions feature is enabled (--langversion:preview).
This implements RFC FS-1043 claim #6: weak resolution should not force
inline code to collapse generic SRTP constraints to concrete types.

Changes:
- CheckExpressions.fs: Skip CanonicalizePartialInferenceProblem for inline
  bindings in TcLetBinding, TcObjectExprBinding, and
  TcIncrementalLetRecGeneralization when ExtensionConstraintSolutions is on.
- IWSAMsAndSRTPsTests.fs: Add 6 tests covering inline DateTime generics,
  non-inline unaffected, backward compat with langversion 8.0, and
  built-in numeric operators.

Non-inline code is completely unaffected. Gated behind langversion:preview.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enhance CreateImplFileTraitContext to search referenced CCU module types
for extension member Vals, not just local impl file bindings. This enables
cross-assembly SRTP constraint resolution during optimization/codegen when
extension operators are defined in a referenced assembly.

Changes:
- ConstraintSolver.CreateImplFileTraitContext: accept CcuThunk list, walk
  referenced CCU module types to collect extension members
- Optimizer.IncrementalOptimizationEnv: add referencedCcus field populated
  by BindCcu
- Optimizer.OptimizeImplFile: pass referencedCcus to CreateImplFileTraitContext
- Tests: change cross-assembly test from shouldFail to compileAndRun/shouldSucceed,
  remove TODO comment, add transitive A→B→C test

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove Skip from 'AllowOverloadOnReturnType resolves overloads by return
type' test now that AllowOverloadOnReturnTypeAttribute is available in
FSharp.Core.

Add test for ambiguity error when no type annotation is present.
Add test for mixed attributed/non-attributed overloads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 tests covering SRTP weak resolution behavior with langversion preview:
- FSharpPlus-style InvokeMap/InvokeApply pattern compiles with preview
- Type annotation workaround for InvokeMap pattern compiles
- Non-inline code canonicalization unaffected by ExtensionConstraintSolutions
- Inline numeric operators with multiple overloads stay generic

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nType assertions

- Remove duplicate 'Non-inline DateTime addition still resolves concretely'
  test (identical to 'Non-inline code canonicalization is unaffected')
- Fix 'AllowOverloadOnReturnType resolves overloads by return type' to assert
  shouldFail (attribute not yet functional for return-type disambiguation)
- Fix 'AllowOverloadOnReturnType mixed' to assert shouldFail (attribute type
  not yet defined in FSharp.Core)

All 227 IWSAM/SRTP tests pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 tests covering instance extension methods with SRTP constraints:
- Instance extension method resolves via SRTP
- Instance extension method with parameter resolves via SRTP
- Instance extension does not satisfy static SRTP constraint
- Intrinsic instance method takes priority over instance extension

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 3 failwith calls with error(InternalError(...)) in ITraitContext
downcast code paths to produce proper compiler diagnostics instead of
opaque exceptions:
- CheckBasics.fs: SelectExtensionMethods InfoReader cast
- ConstraintSolver.fs: SolveMemberConstraint AccessorDomain cast
- ConstraintSolver.fs: GetRelevantMethodsForTrait MethInfo cast

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add 4 new tests covering accessibility-domain scenarios for SRTP:
- Internal type extension in same assembly resolves via SRTP
- SRTP constraints from different accessibility domains flow together
- Internal record field resolves via SRTP within same assembly
- Cross-assembly internal extension is not visible via SRTP

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rename _collectionSettings back to collectionSettings and add early-exit
check after rootResults to skip expensive type hierarchy lookup when only
one result is needed (AtMostOneResult). This restores the perf optimization
that was lost during RFC FS-1043 refactoring.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Document extension members for operators and SRTP constraints in preview release notes
- Add SRTP guide covering extension constraints, priority rules, scope capture, weak resolution changes, and workarounds
- Feature flag: ExtensionConstraintSolutions (--langversion:preview)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove false 'known limitation' about cross-assembly resolution from
  release notes and srtp-guide.md (cross-assembly resolution works)
- Clarify AllowOverloadOnReturnType: attribute allows defining overloads,
  but full call-site disambiguation is not yet implemented
- Add cross-assembly resolution as an explicit feature bullet in release notes
- Fix stale test comment about AllowOverloadOnReturnType not being in FSharp.Core

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add AllowOverloadOnReturnTypeAttribute to all surface area baselines
  (debug ns2.0, debug ns2.1, release ns2.0 were missing the entry)
- Add withErrorCode to all 7 shouldFail tests without error verification:
  - Extension operator not visible: error 1
  - AllowOverloadOnReturnType overloads by return type: error 41
  - Overloads without AllowOverloadOnReturnType: error 41
  - AllowOverloadOnReturnType with no annotation: error 41
  - Instance extension does not satisfy static SRTP: error 1
  - Cross-assembly internal extension not visible: error 43
- Fix mixed overloads test: overloads with different param types (string
  vs int) resolve without ambiguity — changed to shouldSucceed
- Remove AllowOverloadOnReturnType attribute from shouldFail tests since
  the attribute is not available in the test runtime's FSharp.Core
- Parameterize DateTime addition/subtraction tests into a single Theory
- Extract stringRepeatExtLib helper to eliminate duplicated string
  extension operator library definitions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nition

- Merge 'AllowOverloadOnReturnType resolves overloads by return type' and
  'Overloads without AllowOverloadOnReturnType produce ambiguity error'
  into single 'Overloads differing only by return type produce ambiguity
  error' test (identical source and assertions)
- Extract stringRepeatExtDef [<Literal>] constant for inline string
  repeat extension definition, reducing 5 inline duplicates to 1
- Update stringRepeatExtLib to reuse stringRepeatExtDef

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro and others added 10 commits April 16, 2026 18:17
Add test verifying that 'open type StringOps' makes static operators
on StringOps available for SRTP constraint resolution.

Compiler changes:
- NameResolution: add eOpenedTypeOperators field to NameResolutionEnv,
  populated by AddStaticContentOfTypeToNameEnv for operator MethInfos,
  searched by SelectExtensionMethInfosForTrait
- ConstraintSolver: add staticOperatorsByName collection to
  CreateImplFileTraitContext for optimization-phase SRTP resolution
  (operators from open type scopes lose traitCtxt after pickling)
- Filter out operators on support types from staticOperatorsByName to
  avoid changing resolution priority (intrinsics already found)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add single-file and cross-assembly tests verifying that F#-authored
C#-style extension methods using [<Extension>] attribute are correctly
resolved via SRTP constraint resolution.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… via SRTP

Verify that extension members defined in an internal module of a referenced
assembly are not accessible through SRTP constraint resolution, both with
and without optimization enabled. This guards against CreateImplFileTraitContext
(which uses AccessibleFromEverywhere) leaking internal extensions across
assembly boundaries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Negative test: operator on type without it produces FS0001 at compile time
- Positive test: adding extension operator makes SRTP solvable, runs correctly

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ensions

# Conflicts:
#	tests/projects/CompilerCompat/CompilerCompatApp/Program.fs
#	tests/projects/CompilerCompat/CompilerCompatLib/Library.fs
Avoid allocating HashSet<ExtensionMember> and a list comprehension builder
when there are no candidate extension members to consider. Hot under
extension-method / SRTP-heavy code.

Measurement on an fsharpplus-style stress harness
(9600 SRTP sites, 4x call-repetition factor, 5-run median):

  deepest-compiler-frame SelectMethInfosFromExtMembers:
    before: 1033 ms
    after:   714 ms   (-31%)
  TC self time:
    before: 1.33 s  (stdev 0.28)
    after:  1.29 s  (stdev 0.03)

The stdev collapse reflects reduced GC pressure from the per-call
HashSet allocations. No behaviour change: PropertyCollector.Close()
already returns [] on empty input, and both HashSet/ctor allocations
are pure side-effect-free constructions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…to pr-19602

# Conflicts:
#	src/Compiler/Checking/NameResolution.fs
@T-Gro
Copy link
Copy Markdown
Member Author

T-Gro commented Apr 17, 2026

Will continue at #19602 , keep this one for some side quest experiments where I want CI, but do not want to break others.

@T-Gro T-Gro changed the title WIP: RFC FS-1043: Extension members participate in SRTP constraint resolution [Experimental zone for RFC FS-1043: Extension members participate in SRTP constraint resolution] Apr 17, 2026
gusty and others added 17 commits April 18, 2026 21:09
Before: eOpenedTypeOperators was a linear MethInfo list scanned on
every SRTP trait-resolution call in SelectExtensionMethInfosForTrait:
the loop filtered by string equality on minfo.LogicalName, costing
O(N) per lookup where N is the total number of operator methods
brought into scope via 'open type'.

After: store as NameMultiMap<MethInfo> (NameMap<MethInfo list>),
aggregating by LogicalName. Lookup becomes a single NameMultiMap.find,
O(log N) + O(bucket) where bucket size is the number of homograph
operators. Write path folds operatorMethods into the multi-map.

This mirrors the pattern already used for eTyconsByDemangledNameAndArity
and similar fields in NameResolutionEnv. Pure refactor: no change in
semantics, no observable behavior change, no public API or signature
surface impact.

Measured on the small ext-operator SRTP stress: TC wall-clock within
noise (expected — N is small there). Benefit grows linearly with the
number of 'open type' declarations in scope, which is the FSharpPlus-
shaped usage pattern this feature targets.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…peToNameEnv

AddStaticContentOfTypeToNameEnv was calling IntrinsicMethInfosOfType
twice with identical arguments: once for method-group collection and
again for the operator-method filter used to populate
eOpenedTypeOperators. Bind the result once and reuse.

Pure refactor, no behavior change. Called per 'open type' (and per
type opened via 'open' / 'AutoOpen'), so repeated in compilation
units with many such declarations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expert review noted that folding operatorMethods forward into
NameMultiMap reversed within-call source order inside each bucket
(List.fold + prepend), which could subtly affect overload-resolution
diagnostic candidate ordering. Switch to List.foldBack so within-call
order matches the prior list-prepend implementation exactly.

Also extract the indexing step as AddMethInfoByLogicalName, mirroring
the existing AddRecdField helper one NameMap sub-table builder away.
Adversarial review flagged the inline fold as a missed reuse
opportunity against the AddRecdField pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…refactor

Test: OpenTypeOperatorHomographOrder.fs opens a holder type with three
via SRTP. Pins down the >=2-entries-per-bucket path of the new
NameMultiMap<MethInfo> encoding and the foldBack ordering fix.

Also adds a Changed entry to the 11.0.100 compiler-service release notes
covering the three in-commit refactors on this branch.

Verified:
- SurfaceAreaTest passes (no FCS API drift)
- ExtensionConstraintsTests + IWSAMsAndSRTPs + full Conformance.Types suite
  all green (679 tests, 674 passing, 5 skipped)
- fantomas reports no formatting drift on NameResolution.{fs,fsi}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Defn

The ad-hoc grouping of MethInfos by LogicalName in PostInferenceChecks
duplicated the pattern already captured by NameMultiMap.initBy.
Unifies with the eOpenedTypeOperators indexing convention added in
commit 5c44db6d3 (AddMethInfoByLogicalName / NameMultiMap).

- hashOfImmediateMeths and hashOfAllVirtualMethsInParent now built via
  NameMultiMap.initBy.
- hashOfImmediateProps left alone — its incremental build with
  order-dependent self-exclusion is genuinely stateful.
- getHash helper retained for props only.

No semantic change; targeted tests pass; bench-harness perf neutral.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Small, machine-independent slices from PLAN.md, no perf claims.

WS3.1 — Document the order invariant at both ends:
- NameResolution.fs:1283 (List.foldBack site) explains WHY the fold
  direction matters and points to the consumer + the regression test.
- NameResolution.fs:1705 (SelectExtensionMethInfosForTrait) mirrors the
  invariant from the consumer side.

WS4.1 — Expand homograph test matrix from 1 to 4 scenarios:
- OpenTypeOperatorHomographMultipleHolders.fs — operators spread across
  two separate 'open type' holders must all accumulate into the same
  bucket (cross-call bucket accumulation).
- OpenTypeOperatorNestedModule.fs — 'open type' in nested module scopes
  the operator correctly; sibling scope unaffected.
- OpenTypeOperatorShadowing.fs — local 'let inline (op)' shadows the
  extension operator from that point on.

WS4.3 — Release notes rewrite: drop overclaimed perf framing; honestly
describe as 'internal refactor'. Also mention PostInferenceChecks dedup
brought in by the follow-up commit.

WS5.2 — Rename hashOfImmediateMeths -> immediateMethsByLogicalName and
hashOfAllVirtualMethsInParent -> parentVirtualMethsByLogicalName in
CheckEntityDefn. They are NameMultiMaps now, not hashes.

Verified: ExtensionConstraintsTests (22 passed), *SameName* (6),
*Duplicate* (29), *AbstractMember* (18), all green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 4 new ComponentTests covering SRTP dispatch, param-type overloads,
  cross-assembly usage, and CompiledName-renamed operators.
- docs/name-resolution-operators.md explaining eOpenedTypeOperators,
  the order invariant, and consumer paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Broadens regression coverage for duplicate-member diagnostics emitted
by CheckEntityDefn in PostInferenceChecks.fs. Covers:

- Erased-generic-args duplicate method (FS0442)
- Property-vs-method name clash (FS0434)
- Derived-hides-abstract warning (FS0864)
- Indexer-vs-non-indexer property clash (FS0436)
- Getter/setter type mismatch (FS3172)

Prerequisite for further dedup work in CheckEntityDefn (props dict).

(Re-land of lost commit 9146ec08e dropped by a parallel subagent reset.)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The companion NameMultiMap-based indexes (immediateMethsByLogicalName,
parentVirtualMethsByLogicalName) are symmetric full-build + identity-exclusion
at each point. hashOfImmediateProps, by contrast, is deliberately scan-so-far
so that each duplicate PropInfo pair is reported exactly once.

Future readers may be tempted to unify the two patterns — document the
constraint at the site.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document that the open-type extension-operator refactor is structurally
motivated, not perf-motivated: on the FSharpPlus-like stress harness the
delta (1.148s pre / 1.206s post, median of 5) is within noise.

Avoids any implicit perf claim; leaves the door open to corpus-scale
benchmarking (PLAN.md §0.4).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…trim notes

S1 (soften claim): OpenTypeOperatorHomographOrder.fs disambiguates overloads
by parameter type, so it does not actually pin within-call source-order
tie-break. Rewrite both the NameResolution.fs comment and the docs section
to reflect that the invariant is defensive (matches prior list-prepend
semantics; cross-open ordering remains observable), not empirically pinned.

S2 (diagnostic-text fix): In the prior hand-rolled Dictionary build for
parent virtual methods, newer entries were prepended — so
List.tryFind (checkForDup EraseAll) at PostInferenceChecks.fs:2553
returned the *last* matching parent virtual in source order. NameMultiMap
preserves source order, so the same tryFind would return the *first*.
When a base class has >=2 homograph virtuals matching a hiding derived
member, the tcNewMemberHidesAbstractMember warning prints a different
signature. Switch to List.tryFindBack to preserve the prior last-match
selection exactly.

N3 (release notes): trim the 11.0.100.md entry from implementation detail
to one line matching the file's convention; link to the reference doc.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A. anti-duplication + reuse agents flagged: two calls to
   NameMultiMap.initBy (fun (m: MethInfo) -> m.LogicalName) in
   PostInferenceChecks.CheckEntityDefn. Extract a local
   methInfosByLogicalName combinator; both sites now read as one concept.

B. clutter + too-big agents flagged: the NameResolution.fs insertion-site
   comment (12 lines hedging on order-invariant) and its consumer-side
   twin (9 lines) restated the same point twice. Collapse each to 4-5
   lines pointing at the canonical explanation in
   docs/name-resolution-operators.md.

C. clutter agent flagged: the hashOfImmediateProps preamble was a
   prescriptive barrier comment (7 lines arguing against future refactors).
   Trim to a 2-line statement of the scan-so-far property, referencing
   the contrasting immediateMethsByLogicalName.

Intentionally not adopted:
- Parametrizing the 7 [<Fact>] open-type scenario tests into one
  [<Theory>]: the current Facts encode scenario description in the test
  name, which appears verbatim in test output. A Theory with InlineData
  filename would report 'Theory("OpenTypeOperatorShadowing.fs")' and
  lose that.
- Inlining AddMethInfoByLogicalName at its single call site: adversarial
  verdicts split (anti-dup vote keep, too-big vote inline). The helper
  names the intent of the fold and costs one line.
- Trimming the Measurements / Known-gotchas sections of
  docs/name-resolution-operators.md: these are the honest perf record
  and the user-visible language gotchas, not implementation chatter.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Internal notes that don't belong in the shipped diff.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Not a PR into main. Belongs on #19602 if at all.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

3 participants